fix(header): handle drag end without movement - #819
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a column-reorder drag edge case where releasing after the long-press delay but without any pointer movement could leave downstream drag handlers without a usable event payload, breaking subsequent drags. It does this by ensuring drag lifecycle events always include the latest pointer position and by moving reorder target tracking into the header component (removing the orderable directive).
Changes:
- Extend
DatatableDraggableDirectivesodragStart/dragEndemit aDragEvent(including the last known pointer position). - Remove
OrderableDirectiveand shift reorder target detection + drag translation (translateX) intoDataTableHeaderComponent. - Add/adjust unit tests to cover “drag end without movement” and prevent reorder in that scenario.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| projects/ngx-datatable/src/lib/types/internal.types.ts | Removes internal reorder-target-related types/fields that are no longer used after dropping OrderableDirective. |
| projects/ngx-datatable/src/lib/directives/orderable.directive.ts | Removes the directive previously responsible for header reorder targeting and drag subscriptions. |
| projects/ngx-datatable/src/lib/directives/orderable.directive.spec.ts | Removes tests tied specifically to the deleted OrderableDirective. |
| projects/ngx-datatable/src/lib/directives/draggable.directive.spec.ts | Updates dragEnd typing and adds coverage for drag end without movement including initial pointer position. |
| projects/ngx-datatable/src/lib/directives/datatable-draggable.directive.ts | Ensures drag lifecycle events emit a consistent DragEvent payload (including on drag end without movement). |
| projects/ngx-datatable/src/lib/components/header/header.component.ts | Handles drag start/move/end directly via template bindings; computes reorder target via elementsFromPoint; uses translateX for visual movement and resets at end. |
| projects/ngx-datatable/src/lib/components/header/header.component.spec.ts | Adds coverage to ensure no reorder occurs when drag ends without pointer movement and transform is cleared. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fh1ch
approved these changes
Aug 25, 2026
fh1ch
left a comment
Member
There was a problem hiding this comment.
@spike-rabbit all good, thanks a lot 🙇
LGTM 👍
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce? (check one with "x")
What is the current behavior? (You can also link to an open issue here)
Holding a reorderable column header until dragging starts and releasing it without moving leaves no drag-move event for the orderable directive. Drag end then attempts to destructure an undefined event and breaks subsequent column dragging.
Closes #790
What is the new behavior?
Every drag lifecycle event carries the latest pointer position, including drag end when the pointer never moved. The header handles drag events through normal template bindings and owns reorder target state directly, removing the descendant output subscriptions and the orderable directive. Column movement uses a direct
translateXtransform and is cleared when dragging ends.Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: N/A
Other information:
Note: This is a backport of the original PR #797.